Fix destroying k8s cluster on shared networks by Pearl1594 · Pull Request #4461 · apache/cloudstack

Pearl1594 · 2020-11-11T09:45:28Z

Description

Deletion of K8s cluster on shared networks fails due to attempt to clear network rules, however, no network rules get added for clusters brought up on Shared networks.

2020-11-10 16:46:26,125 WARN  [c.c.k.c.a.KubernetesClusterActionWorker] (API-Job-Executor-7:ctx-46554d1f job-67 ctx-cf887921) (logid:ed80132a) Failed to remove network rules of Kubernetes cluster : c1
com.cloud.exception.ManagementServerException: No source NAT IP addresses found for network : shared1
        at com.cloud.kubernetes.cluster.actionworkers.KubernetesClusterDestroyWorker.deleteKubernetesClusterNetworkRules(KubernetesClusterDestroyWorker.java:145)
        at com.cloud.kubernetes.cluster.actionworkers.KubernetesClusterDestroyWorker.destroy(KubernetesClusterDestroyWorker.java:242)
        at com.cloud.kubernetes.cluster.KubernetesClusterManagerImpl.deleteKubernetesCluster(KubernetesClusterManagerImpl.java:1144)
        at org.apache.cloudstack.api.command.user.kubernetes.cluster.DeleteKubernetesClusterCmd.execute(DeleteKubernetesClusterCmd.java:77)
        at com.cloud.api.ApiDispatcher.dispatch(ApiDispatcher.java:156)
        at com.cloud.api.ApiAsyncJobDispatcher.runJob(ApiAsyncJobDispatcher.java:108)
        at org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl$5.runInContext(AsyncJobManagerImpl.java:620)
        at org.apache.cloudstack.managed.context.ManagedContextRunnable$1.run(ManagedContextRunnable.java:48)
        at org.apache.cloudstack.managed.context.impl.DefaultManagedContext$1.call(DefaultManagedContext.java:55)
        at org.apache.cloudstack.managed.context.impl.DefaultManagedContext.callWithContext(DefaultManagedContext.java:102)
        at org.apache.cloudstack.managed.context.impl.DefaultManagedContext.runWithContext(DefaultManagedContext.java:52)
        at org.apache.cloudstack.managed.context.ManagedContextRunnable.run(ManagedContextRunnable.java:45)
        at org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl$5.run(AsyncJobManagerImpl.java:568)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

How Has This Been Tested?

Deployed a cluster on a shared network and then cleaned it up

Pearl1594 · 2020-11-11T09:45:52Z

@blueorangutan package

blueorangutan · 2020-11-11T09:46:21Z

@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-11-11T10:27:27Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2365

DaanHoogland · 2020-11-11T10:59:04Z

+                    NetworkVO kubernetesClusterNetwork = networkDao.findById(kubernetesCluster.getNetworkId());
+                    if (kubernetesClusterNetwork != null && kubernetesClusterNetwork.getGuestType() != Network.GuestType.Shared) {
+                        deleteKubernetesClusterNetworkRules();
+                    }


I know I'm not a nice guy, but can we please have new code in separate methods:

private void checkForRulesToDelete(){ NetworkVO kubernetesClusterNetwork = networkDao.findById(kubernetesCluster.getNetworkId()); if (kubernetesClusterNetwork != null && kubernetesClusterNetwork.getGuestType() != Network.GuestType.Shared) { deleteKubernetesClusterNetworkRules(); } }

and then

Suggested change

NetworkVO kubernetesClusterNetwork = networkDao.findById(kubernetesCluster.getNetworkId());

if (kubernetesClusterNetwork != null && kubernetesClusterNetwork.getGuestType() != Network.GuestType.Shared) {

deleteKubernetesClusterNetworkRules();

}

checkForRulesToDelete();

Pearl1594 · 2020-11-11T14:40:34Z

@blueorangutan package

blueorangutan · 2020-11-11T14:41:22Z

@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-11-11T14:47:07Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2369

blueorangutan · 2020-11-11T15:15:39Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2371

Pearl1594 · 2020-11-11T15:23:27Z

@blueorangutan test

blueorangutan · 2020-11-11T15:24:27Z

@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2020-11-12T00:37:24Z

Trillian test result (tid-3158)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31254 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4461-t3158-kvm-centos7.zip
Smoke tests completed. 86 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland · 2020-11-17T18:56:17Z

@Pearl1594 is this really minor? k8 would not work at all in shared nets because of this would it? cc @PaulAngus
@shwstppr can you review this please?

shwstppr

LGTM

Pearl1594 · 2020-11-18T04:35:24Z

@DaanHoogland It isn't a critical bug - and can be pushed to the next milestone if need be. It basically looks after the scenario wherein when a cluster deployed in a shared network doesn't get cleaned up / garbage collected and indefinitely stays in destroying state

shwstppr · 2020-11-18T06:18:47Z

@Pearl1594 @DaanHoogland I think this should go to 4.14.1.0

DaanHoogland · 2020-11-18T07:28:27Z

@Pearl1594 can you rebase on 4.14?

Pearl1594 · 2020-11-18T09:58:37Z

Done @DaanHoogland

DaanHoogland · 2020-11-18T11:28:02Z

@blueorangutan package

blueorangutan · 2020-11-18T11:28:57Z

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-11-18T12:16:55Z

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2398

DaanHoogland · 2020-11-18T12:23:43Z

@blueorangutan test

blueorangutan · 2020-11-18T12:25:03Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

DaanHoogland

code looks good

blueorangutan · 2020-11-18T22:07:41Z

Trillian test result (tid-3196)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32934 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4461-t3196-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

yadvr · 2020-11-18T23:20:54Z

Lgtm

yadvr · 2020-11-18T23:21:40Z

This fixes a valid case for a supported feature, let's merge

Pearl1594 requested a review from shwstppr November 11, 2020 09:45

Pearl1594 requested a review from DaanHoogland November 11, 2020 09:46

DaanHoogland reviewed Nov 11, 2020

View reviewed changes

DaanHoogland added component:kubernetes Severity:Critical Critical bug Severity:Minor type:bug and removed Severity:Critical Critical bug labels Nov 12, 2020

DaanHoogland added this to the 4.16.0.0 milestone Nov 17, 2020

DaanHoogland added the NEEDS-GO-NO_GO-URGENTLY label Nov 17, 2020

DaanHoogland modified the milestones: 4.16.0.0, 4.15.0.0 Nov 17, 2020

shwstppr approved these changes Nov 17, 2020

View reviewed changes

Pearl Dsilva added 2 commits November 18, 2020 15:26

Fix destroying k8s cluster on shared networks

b0bceaa

Extracted code

7b09f3c

Pearl1594 force-pushed the fix-k8s-deletion branch from cf2e011 to 7b09f3c Compare November 18, 2020 09:57

Pearl1594 changed the base branch from master to 4.14 November 18, 2020 09:58

DaanHoogland modified the milestones: 4.15.0.0, 4.14.1.0 Nov 18, 2020

DaanHoogland approved these changes Nov 18, 2020

View reviewed changes

yadvr merged commit 1692df4 into apache:4.14 Nov 18, 2020

Conversation

Pearl1594 commented Nov 11, 2020

Description

Types of changes

How Has This Been Tested?

Uh oh!

Pearl1594 commented Nov 11, 2020

Uh oh!

blueorangutan commented Nov 11, 2020

Uh oh!

blueorangutan commented Nov 11, 2020

Uh oh!

DaanHoogland Nov 11, 2020

Choose a reason for hiding this comment

Uh oh!

Pearl1594 commented Nov 11, 2020

Uh oh!

blueorangutan commented Nov 11, 2020

Uh oh!

blueorangutan commented Nov 11, 2020

Uh oh!

blueorangutan commented Nov 11, 2020

Uh oh!

Pearl1594 commented Nov 11, 2020

Uh oh!

blueorangutan commented Nov 11, 2020

Uh oh!

blueorangutan commented Nov 12, 2020

Uh oh!

DaanHoogland commented Nov 17, 2020

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

Pearl1594 commented Nov 18, 2020

Uh oh!

shwstppr commented Nov 18, 2020

Uh oh!

DaanHoogland commented Nov 18, 2020

Uh oh!

Pearl1594 commented Nov 18, 2020

Uh oh!

DaanHoogland commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

DaanHoogland commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

yadvr commented Nov 18, 2020

Uh oh!

yadvr commented Nov 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants